Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Encode non-ascii characters properly when sending 2fa emails #8

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Encode non-ascii characters properly when sending 2fa emails #8

wants to merge 2 commits into from

Conversation

kadamwhite
Copy link

@kadamwhite kadamwhite commented Jan 16, 2023 •
edited
Loading

Draft PR, not ready for merge yet. Attempted this as-is in #7 and had to revert due to an error:

Uncaught ValueError: mb_encode_mimeheader(): Argument #2 ($charset) must be a valid encoding, "3" given in /usr/src/app/vendor/humanmade/two-factor/providers/class.two-factor-email.php:151
Stack trace:
#0 /usr/src/app/vendor/humanmade/two-factor/providers/class.two-factor-email.php(151): mb_encode_mimeheader('Blo...', '3')
#1 /usr/src/app/vendor/humanmade/two-factor/providers/class.two-factor-email.php(171): Two_Factor_Email->generate_and_email_token(Object(WP_User))
#2 /usr/src/app/vendor/humanmade/two-factor/class.two-factor-core.php(361): Two_Factor_Email->authentication_page(Object(WP_User))
#3 /usr/src/app/vendor/humanmade/two-factor/class.two-factor-core.php(303): Two_Factor_Core::login_html(Object(WP_User), '0d498aca32a1c7d...', 'https://site...', '', Object(Two_Factor_Email))
#4 /usr/src/app/wordpress/wp-includes/class-wp-hook.php(307): Two_Factor_Core::backup_2fa('')
#5 /usr/src/app/wordpress/wp-includes/class-wp-hook.php(331): WP_Hook->apply_filters('', Array)
#6 /usr/src/app/wordpress/wp-includes/plugin.php(474): WP_Hook->do_action(Array)
#7 /usr/src/app/wordpress/wp-login.php(518): do_action('login_form_back...')
#8 {main}
  thrown

@ajvillegas mentioned,

The mb_encode_mimeheader function is expecting the charset as the second parameter, but I left the ENT_QUOTES flag from the wp_specialchars_decode we had previously on the PR and think this is what's causing the error. It looks like the second parameter should be UFT-8.

This PR makes that change.

@kadamwhite
Copy link
Author

kadamwhite commented Jan 16, 2023 •
edited
Loading

Limited testing indicates that this fixes the subject but not the sender field. Maybe we need to filter the get_option for blogname, to apply this within the option accessor?

add_filter( 'option_blogname', function( $blogname ) {
    return mb_encode_mimeheader( $blogname, 'UTF-8' );
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@kadamwhite